Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

download aritfact from latest run which upload an artifact #88

Merged
merged 9 commits into from
Aug 11, 2021
Merged

download aritfact from latest run which upload an artifact #88

merged 9 commits into from
Aug 11, 2021

Conversation

shitlsh
Copy link
Contributor

@shitlsh shitlsh commented Jul 6, 2021

Hi, I have the same problem with this issue:#46.
Seeing as words is difficult to elaborate the problem, I try to submit this PR to see if you can understand what we want.
Cause I'm not familiar with the Github Action API, the code change maybe not correct.
There are something I want to explain:

  1. I have limited the response per_page to 10, because I find that asyncFind will request with each run_id
  2. I don't know how to add a auto test to confirm my change

@dawidd6 dawidd6 self-requested a review July 6, 2021 16:27
@dawidd6 dawidd6 self-assigned this Jul 8, 2021
@dawidd6
Copy link
Owner

dawidd6 commented Jul 13, 2021

I'm gonna wait till this PR is merged: #90

Then I'll look at this one.

@dawidd6 dawidd6 removed their request for review July 13, 2021 18:13
@dawidd6
Copy link
Owner

dawidd6 commented Aug 4, 2021

Solution you provided increases the number of API calls per execution and that number can be high in some cases. I'm not really convinced.

@shitlsh
Copy link
Contributor Author

shitlsh commented Aug 5, 2021 •
edited

Yes, you're right.But apparently Github doesn't provide a better way to make it.What if I add a toggle on this feature?
Something like this

# Optional, will get the last available artifact from previous workflow, default false, just try to download from the last one
(can you decide the name):  false

@dawidd6
Copy link
Owner

dawidd6 commented Aug 5, 2021

Yes, a feature flag would be nice. check_artifacts maybe? I'm open for suggestions on this one. Also please rebase on latest master as there were changes to this part of code and avoid creating functions, keep it simple.

@shitlsh
Copy link
Contributor Author

shitlsh commented Aug 7, 2021

Sorry for late reply and thanks for your reminder. I'll set aside some time next week to finish this PR.

@shitlsh
Copy link
Contributor Author

shitlsh commented Aug 11, 2021 •
edited

I have fetched upstream and rebased the latest changes. I saw you have optimized the loop of run, so I followed this change and made my feature more simple.

@dawidd6
Copy link
Owner

dawidd6 commented Aug 11, 2021

Nice. Thanks

@dawidd6 dawidd6 merged commit d0f291c into dawidd6:master Aug 11, 2021
sscott1-godaddy added a commit to gdcorp-action-public-forks/action-download-artifact that referenced this pull request Sep 16, 2021
@sscott1-godaddy @nickthegroot
@dawidd6 @erd0s @shitlsh
Merge upstream changes (#10)
7455d28 
* Add pagination when listing all artifacts (dawidd6#90)

Co-authored-by: Nick DeGroot <1966472+nickthegroot@users.noreply.github.com>

* main: remove ';'

* README: fix

* main: throw if no runID

* action: only success

* README: update

* workflows: test empty conclusion

* Fix octokit returning weird results from listWorkflowRuns (dawidd6#95)

Closes: dawidd6#93 

Co-authored-by: Dawid Dziurla <dawidd0811@gmail.com>

* README: remove stray dot

* download aritfact from latest run which upload an artifact (dawidd6#88)

Co-authored-by: Nick DeGroot <nbdegroot1@gmail.com>
Co-authored-by: Nick DeGroot <1966472+nickthegroot@users.noreply.github.com>
Co-authored-by: Dawid Dziurla <dawidd0811@gmail.com>
Co-authored-by: Dirk <dirks@seatfrog.com>
Co-authored-by: Tailong <54169577+shitlsh@users.noreply.github.com>
@dawidd6 dawidd6 mentioned this pull request Nov 22, 2021
Merged
@gustavovalverde gustavovalverde mentioned this pull request Mar 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dawidd6 dawidd6 dawidd6 approved these changes

Assignees

@dawidd6 dawidd6

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@shitlsh @dawidd6